Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bring back BART to V4 and make it more general #4914

Merged
merged 26 commits into from
Sep 3, 2021
Merged

Bring back BART to V4 and make it more general #4914

merged 26 commits into from
Sep 3, 2021

Conversation

aloctavodia
Copy link
Member

@aloctavodia aloctavodia commented Aug 9, 2021 •
edited
Loading

This PR brings BART to V4, including some of the features in V3 (unreleased) plus a couple of changes not in V3 in order to make BART more general. For example It should now be possible to create semi-parametric models like:

with pm.Model() as model:
    μ = pm.BART("μ", X, Y)
    α = pm.Normal("α", 0, 1)
    β = pm.Normal("β", 0, 1)
    σ = pm.HalfNormal('σ', 1)
    y = pm.Normal("y", μ+α+β*X, σ, observed=Y)

There was a broadcasting problem when mixing NUTS and PGBART, that was actually a bug in base_hmc. It was finally fixed in #4952

@codecov
Copy link

codecov bot commented Aug 9, 2021 •
edited
Loading

Codecov Report

Merging #4914 (d67c9a3) into main (9cf800b) will increase coverage by 2.25%.
The diff coverage is 96.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4914      +/-   ##
==========================================
+ Coverage   74.08%   76.33%   +2.25%     
==========================================
  Files          86       86              
  Lines       13862    13925      +63     
==========================================
+ Hits        10270    10630     +360     
+ Misses       3592     3295     -297
Impacted Files Coverage Δ
pymc3/step_methods/hmc/base_hmc.py 91.05% <ø> (ø)
pymc3/step_methods/pgbart.py 95.80% <94.59%> (+77.67%) ⬆️
pymc3/distributions/bart.py 100.00% <100.00%> (+81.94%) ⬆️
pymc3/distributions/tree.py 100.00% <100.00%> (+69.13%) ⬆️
pymc3/sampling.py 86.00% <100.00%> (+0.32%) ⬆️
pymc3/step_methods/hmc/nuts.py 97.50% <100.00%> (ø)
pymc3/smc/smc.py 98.37% <0.00%> (-0.34%) ⬇️
pymc3/aesaraf.py 91.26% <0.00%> (ø)
pymc3/distributions/__init__.py 100.00% <0.00%> (ø)
pymc3/distributions/continuous.py 96.26% <0.00%> (+0.02%) ⬆️
... and 7 more

@aloctavodia aloctavodia changed the title [WIP] bring back BART to V4 and make it more general Bring back BART to V4 and make it more general Aug 10, 2021
ricardoV94
ricardoV94 reviewed Aug 17, 2021
View reviewed changes
pymc3/distributions/bart.py Outdated Show resolved Hide resolved
pymc3/distributions/bart.py Outdated Show resolved Hide resolved
pymc3/sampling.py Outdated Show resolved Hide resolved
pymc3/step_methods/hmc/nuts.py Show resolved Hide resolved
@aloctavodia aloctavodia mentioned this pull request Aug 19, 2021
Merged
@aloctavodia aloctavodia force-pushed the unibart branch 2 times, most recently from 4263083 to 4d8696f Compare August 23, 2021 12:17
@aloctavodia
Copy link
Member Author

This is ready to merge @ricardoV94 @twiecki?

ricardoV94
ricardoV94 reviewed Sep 3, 2021
View reviewed changes
Copy link
Member

@ricardoV94 ricardoV94 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but it seems some important branches in the rng_fn are not reached by the tests: https://github.com/pymc-devs/pymc3/pull/4914/files#annotation_1841363883

@aloctavodia aloctavodia merged commit ff45994 into pymc-devs:main Sep 3, 2021
@aloctavodia aloctavodia deleted the unibart branch September 3, 2021 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardoV94 ricardoV94 ricardoV94 left review comments

@twiecki twiecki twiecki approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aloctavodia @twiecki @ricardoV94